Forward telemetry config to kernel backend - #451
Conversation
There was a problem hiding this comment.
Verdict: 1 Medium
Looks good overall — the telemetry-skip logic, kernel config forwarding, and unit coverage are coherent and the doc updates match the new behavior. One medium maintainability concern: removing the RecordConnectionConfig call leaves kernel_telemetry.go (and its helpers) as dead production code reachable only from its test.
Other findings
- 🟡 Medium — This PR removes the sole production caller of
kernelConnectionTelemetry— theconn.telemetry.RecordConnectionConfig(ctx, conn.id, kernelConnectionTelemetry(c.cfg))block that previously lived inconnector.go. After this change,kernelConnectionTelemetry(and its transitive helperskernelUsesProxy/kernelAuthMechthat are only reached through it) are referenced only fromkernel_telemetry_test.go. That makes the wholekernel_telemetry.gofile dead production code: it builds theDriverConnectionParameterstelemetry payload that nothing emits anymore, andstaticcheck's unused-code check won't flag it because the test still references it.
Since the kernel path now skips the Go telemetry interceptor entirely, this connection-config payload is never produced. Either delete kernel_telemetry.go (and its test), or, if it's being kept as scaffolding for the future kernel handoff, add a comment stating that and note it's intentionally test-only for now. As-is it reads as live code and will mislead the next reader into thinking kernel connection-config telemetry is still emitted.
Summary
Tests
Note: the current kernel C ABI does not yet expose telemetry setters, so this PR wires and tests the Go-side config shape for the future kernel handoff without adding a non-linkable cgo call.